Skip to content

Conversation

necolas
Copy link
Contributor

@necolas necolas commented Aug 4, 2025

We shouldn't create an object if the underlying node is null, as this handle is meant to emulate the node but can't proxy methods if none exist.

@necolas necolas requested a review from martinbooth August 4, 2025 23:59
@meta-cla meta-cla bot added the cla signed label Aug 4, 2025
Copy link

github-actions bot commented Aug 5, 2025

workflow: benchmarks/size

Comparison of minified (terser) and compressed (brotli) size results, measured in bytes. Smaller is better.

Results Base Patch Ratio
react-strict-dom/dist/dom/index.js
· compressed 2,514 2,514 1.00
· minified 8,691 8,691 1.00
react-strict-dom/dist/dom/runtime.js
· compressed 855 855 1.00
· minified 2,435 2,435 1.00
react-strict-dom/dist/native/index.js
· compressed 16,339 16,445 1.01 +
· minified 62,604 63,057 1.01 +

Copy link

github-actions bot commented Aug 5, 2025

workflow: benchmarks/perf (native)

Comparison of performance test results, measured in operations per second. Larger is better.

Results Base Patch Ratio
css.create
· small 1,135,144 1,150,851 1.01 +
· small with units 488,924 496,863 1.02 +
· small with variables 652,083 664,489 1.02 +
· several small 349,828 357,504 1.02 +
· large 203,882 203,079 1.00 -
· large with polyfills 146,974 148,855 1.01 +
· complex 103,057 103,279 1.00 +
· unsupported 214,405 211,132 0.98 -
css.createTheme
· simple theme 227,748 228,260 1.00 +
· polyfill theme 212,185 212,623 1.00 +
css.props
· small 225,011 227,098 1.01 +
· small with units 187,354 187,219 1.00 -
· small with variables 111,981 112,294 1.00 +
· small with variables of units 81,879 82,263 1.00 +
· large 94,392 95,595 1.01 +
· large with polyfills 37,818 38,368 1.01 +
· complex 22,222 22,301 1.00 +
· unsupported 138,237 140,081 1.01 +
· simple merge 156,979 157,898 1.01 +
· wide merge 16,930 16,916 1.00 -
· deep merge 16,581 16,685 1.01 +
· themed merge 32,763 32,592 0.99 -

@necolas necolas force-pushed the native/null-handle branch from 3cb0a57 to 083f2a6 Compare August 5, 2025 00:34
Comment on lines +46 to +117
const elementCallback = useElementCallback(
React.useCallback(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need useElementCallback here.

The useCallback already has ref as a dependency, which means the function returned by useCallback will already be invalidated if ref changes. And when that new function is passed into the native component's ref prop below, it will naturally detach the previous one and attach the new one.

@necolas necolas force-pushed the native/null-handle branch from 083f2a6 to 95ab62e Compare August 5, 2025 01:01
@necolas necolas changed the title useImperativeHandle returns null if node is null Go back to using ref callback instead of useImperativeHandle Aug 5, 2025
We shouldn't create an object if the underlying node is null, as this
handle is meant to emulate the node but can't proxy methods if none
exist. Since useImperativeHandle isn't reset when the underlying node
changes, we go back to using ref callbacks. Rather than mutating the
underling node, we create a new object that wraps the relevant fields.
@necolas necolas force-pushed the native/null-handle branch from 95ab62e to d9f8e39 Compare August 5, 2025 01:06
@necolas necolas merged commit d9f8e39 into main Aug 7, 2025
8 checks passed
@necolas necolas deleted the native/null-handle branch September 25, 2025 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants